Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add test for relationship filter with parent(parent(field)) #457

Closed
wants to merge 1 commit into from

Conversation

vonagam
Copy link
Contributor

@vonagam vonagam commented Jan 10, 2025

The test currently fails.

parent(parent(id)) behaves as parent(id).

@zachdaniel
Copy link
Contributor

The fix for this is unfortunately complex. What should be done to resolve this in the general case is to not use simple integers for the binding names, but to use something like {identifier, num}. actually typing that out it might not be too hard, but it will take enough time that I don't think I can do it in the near future.

Please open an issue to track this work. If you or someone else would like to do it, the general pattern would be to:

  1. generate a unique id for each query we generate. We can use something like System.monotonic_integer, or even make_ref. That parts not important really.
  2. when we bind something, we add the query's id to the binding, i.e {id, binding}
  3. Whenever we do a lateral join, we ensure that the query we are lateral joining to has its own identifier

@zachdaniel
Copy link
Contributor

Please remind me when you open the issue to add it to the roadmap under the "soon" category.

@zachdaniel
Copy link
Contributor

In the meantime, you can potentially accomplish what you need to accomplish with manual relationships or custom calculations.

@vonagam
Copy link
Contributor Author

vonagam commented Jan 12, 2025

Ok, opened the issue.

Yeah, I was using fragment workaround but thought about converting it into argumented calculation.

(Reminder about adding "soon" category to the issue.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants